Skip to content

Fix: accept an AuthScheme at the OpenAPI tool entry points - #774

Open
AmaadMartin wants to merge 3 commits into
mainfrom
fix/openapi-tool-widen-auth-scheme-type
Open

Fix: accept an AuthScheme at the OpenAPI tool entry points#774
AmaadMartin wants to merge 3 commits into
mainfrom
fix/openapi-tool-widen-auth-scheme-type

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A

  2. Or, if no issue exists, describe the change:

Problem: The credential machinery under the OpenAPI tool layer is typed against AuthScheme, which is OpenAPIV3.SecuritySchemeObject | OpenIdConnectWithConfig. The OpenAPI tool entry points still declared the narrower SecuritySchemeObject. getTokenEndpoint and determineGrantType read the OpenIdConnectWithConfig endpoint fields at runtime, so the feature works, but a caller cannot pass the scheme inline. new OpenAPIToolset({specStr, authScheme: {type: 'openIdConnect', authorizationEndpoint: ..., tokenEndpoint: ...}}) fails to compile with TS2353.

Solution: I widened the ten positions where a caller supplies a scheme to AuthScheme. This matches adk-python, where each of these is already Optional[AuthScheme]. Every change is a parameter or field widening, so no existing call site breaks and no runtime behaviour changes. The diff is types and imports only.

Three sites keep SecuritySchemeObject on purpose, because they are output positions where widening costs callers precision and unlocks no new call:

  • openapi_spec_parser.ts:228 — the spec.components?.securitySchemes cast. These values come from an OpenAPIV3.Document, which cannot express an OpenIdConnectWithConfig.
  • auth_helpers.ts:64,75 — the createApiKeyScheme and createBearerScheme return types. Their results stay assignable to every widened parameter by subtyping.

Out of scope, and deliberately not done: adk-python's dict_to_auth_scheme normalisation, any CustomAuthScheme, and any change to how the credential cache key is derived.

Collision check: I listed all 400 open and closed pull requests on the fork and read the file lists of the nine that touch openapi_tool or auth. None changes an auth-scheme type, so I branched from main rather than stacking. The nearest neighbours are #772 (tool_auth_handler.ts), #605 (openapi_toolset.ts), #750, #748, #742 and #645 (the same test files). Any conflict with them is textual.

Note on #645: that pull request replaces the private authScheme field reads in openapi_toolset_test.ts with public-API assertions. My new cases therefore observe the scheme through the public path — the ToolAuthHandler.fromToolContext call and the configureAuthScheme override — instead of adding more private field reads.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

I added 7 cases across 5 files. I added new it(...) blocks and edited no existing test.

npx vitest run --project unit:core core/test/tools/openapi_tool/
  Test Files  8 passed (8)
       Tests  89 passed (89)

Read this before the rest: tsc is the only guard, and CI does not run it.

This change is types-only. TypeScript erases types, so no runtime test can tell the code before this change from the code after it. All 7 new cases pass on main unchanged. The regression guard is npm run ts:check and nothing else.

That guard is not enforced by CI today. I checked:

  • .github/workflows/validation.yaml runs build, test:coverage, lint, format:check and docs:check. It does not run ts:check.
  • core/tsconfig.json sets "include": ["src/**/*"], so npm run build never typechecks core/test.
  • vitest.config.ts sets no typecheck option.
  • typedoc.json excludes **/*_test.ts, so docs:check does not see these files either.

So if someone narrows one of these signatures again, CI stays green. A reviewer should treat the ts:check output below as the evidence, not the green checkmarks on this PR.

I did not wire ts:check into CI here. It is not green today: it reports 291 pre-existing errors across core/test and tests/, and several open pull requests are already working through them. Turning it on is a separate job, much larger than this change.

Every new case uses an inline object literal at the call site. This matters. OpenIdConnectWithConfig extends OpenAPIV3.OpenIdSecurityScheme, so a value already typed as either one is structurally assignable to SecuritySchemeObject and compiles against the old narrow signature too. Only a fresh object literal trips the excess-property check. A test that assigns to a typed const first would pass before and after this change and would prove nothing.

Proof the compile-time signal works. I reverted the five signatures to OpenAPIV3.SecuritySchemeObject, rebuilt, and ran npm run ts:check. Each of the five files reports the error, 8 in total:

core/test/.../auth_helpers_test.ts:116:9               - error TS2353
core/test/.../openapi_toolset_integration_test.ts:49:9 - error TS2353
core/test/.../openapi_toolset_test.ts:146:9            - error TS2353
core/test/.../rest_api_tool_test.ts:662:9              - error TS2353
core/test/.../rest_api_tool_test.ts:692:7              - error TS2353
core/test/.../rest_api_tool_test.ts:750:11             - error TS2353
core/test/.../tool_auth_handler_test.ts:261:9          - error TS2353
core/test/.../tool_auth_handler_test.ts:293:9          - error TS2353

Object literal may only specify known properties,
and 'authorizationEndpoint' does not exist in type 'OpenIdSecurityScheme'.

Proof the new cases are not vacuous. These mutations do not distinguish this PR's before and after — nothing can, as explained above. They show the assertions pin real behaviour rather than passing regardless. I mutated the exact line each case pins and confirmed a failure:

Mutation Case that failed
getCredentialKey returns a hard-coded 'default' derives the same credential-store key ... — expected undefined to be 'exchanged-token'
fromToolContext passes undefined for authScheme accepts an OpenID Connect scheme ... via fromToolContext
configureAuthScheme becomes a no-op hands an OpenID Connect scheme ... to the auth handler
createRestApiTool drops parsed.authScheme createRestApiTool > accepts an OpenID Connect scheme ...
OpenAPIToolset drops the configureAuthScheme override global auth override case, and the integration case
applyCredential writes a different default header applies an API key credential when given an OpenID Connect scheme ...

ParsedOperation.authScheme gets no new test. It is an output position, so no call site can demonstrate it. Its proof is that the existing assertions on it still compile and pass unmodified.

Coverage. Type annotations are erased at compile time, so this change adds no executable lines. Coverage does not regress.

No suppressions. I added no @ts-expect-error, @ts-ignore, eslint-disable, as any or as never. npm run ts:check reports 291 errors on this branch and 291 on main, and the per-file counts are identical, so this change introduces none.

Manual End-to-End (E2E) Tests:

To reproduce the original failure, check out main and compile this:

new OpenAPIToolset({
  specStr,
  authScheme: {
    type: 'openIdConnect',
    openIdConnectUrl:
      'https://issuer.example.com/.well-known/openid-configuration',
    authorizationEndpoint: 'https://issuer.example.com/authorize',
    tokenEndpoint: 'https://issuer.example.com/token',
  },
});

npm run ts:check rejects it on main and accepts it on this branch. core/test/tools/openapi_tool/openapi_toolset_integration_test.ts runs that exact flow through the public @google/adk entry point, building the toolset from a spec string and asserting the override reaches every tool.

Commands run on the pushed commit:

npm run build        # pass
npm run ts:check     # 291 errors, byte-identical to main
npm run lint         # pass
npm run format:check # pass

tests/e2e/tools/rest_api_tool_auth_e2e_test.ts is unmodified and is the regression check that behaviour did not change. It does not run in this environment: it needs a real model. It fails with API key must be provided via constructor or GOOGLE_GENAI_API_KEY or GEMINI_API_KEY environment variable on this branch and identically on the base commit 9360bf24, so the failure is environmental and unrelated.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

Amaad Martin added 3 commits August 7, 2026 06:13
The credential machinery under the OpenAPI tool layer is typed against
AuthScheme, which adds OpenIdConnectWithConfig to SecuritySchemeObject.
The tool layer still declared the narrower SecuritySchemeObject, so a
caller could not pass an OIDC-with-config literal that the exchangers
read at runtime.

Widen the ten input positions to AuthScheme. Output positions keep
SecuritySchemeObject so callers lose no precision.
Each case passes an inline OpenID Connect literal that carries endpoint
config. An inline literal is what the excess-property check rejects, so
reverting a signature makes tsc report TS2353 on these lines. A value
first assigned to a typed const compiles either way and proves nothing.

The cases observe the scheme through the public auth path -- the
ToolAuthHandler.fromToolContext call and the configureAuthScheme
override -- rather than reading the private authScheme field.

The tool_auth_handler case also pins the credential cache key, which
must stay openIdConnect_existing_exchanged_credential.
An in-body mockRestore() never runs when an assertion above it throws,
which is the case that matters. The spy then leaks into every later
test in the file. Move the restore into afterEach, matching the hook
rest_api_tool_test.ts already uses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant